Conversation
|
@microsoft-github-policy-service agree |
|
Cross-posting here: #30 (comment) |
src/sys/unix.rs
Outdated
| unsafe extern "C" { | ||
| fn __error() -> *mut libc::c_int; | ||
| } |
There was a problem hiding this comment.
You can use libc::__error() instead, I believe. Also, this should be similarly abstracted away:
#[cfg(target_os = "macos")]
pub fn get_errno() -> libc::c_int {
unsafe { *libc::__error() };
}
#[cfg(not(target_os = "macos"))]
pub fn get_errno() -> libc::c_int {
unsafe { *libc::__errno_location() };
}Then you can use it in place of the other calls in this file. This function should go to the end of the file, near the other errno related functions.
There was a problem hiding this comment.
This should probably be changed to io::Error::last_os_error() as the cross platform way to do this (sources: https://users.rust-lang.org/t/how-to-get-errno-value-from-rust-libc/85224/9
https://doc.rust-lang.org/std/io/struct.Error.html#method.last_os_error)
Learned about this elsewhere
There was a problem hiding this comment.
This is really nice and also compiles nicely in release mode, but under opt-level=s the last_os_error call doesn't get properly inlined and this leaves quite a lot of code around. I'll see if there's a workaround...
|
If wanted I can pick this PR up tomorrow if needed and finish the latest requested changes. |
|
I confirm this is working with only modifications applied to
Search function doesn't seem to work though:
|
|
@factormystic I pushed a few commits into your PR. It'd be nice if you could check them out. It'd be really kind if someone could test these latest changes. |
|
Works for me (macOS/ARM64). Search function still doesn't work: complains about missing |
|
I pulled your changes and it seems to work fine for me on macOS 👍 |
|
The issue with ICU was also discussed in #52. I've internally requested a Mac for testing this myself, but I don't live in the US so it may be difficult to get one loaned out. |
|
You should be able to link in |
DHowett
left a comment
DHowett
left a comment
There was a problem hiding this comment.
This looks good to me, thanks!
|
As a treat, here it is on netbsd
|
Fixes microsoft#66 Fixes microsoft#124 Still doesn't have color in Terminal.app on macOS, though. Co-authored-by: Leonard Hecker <leonard@hecker.io>
8 participants
Fixes #66. Still doesn't have color in Terminal.app, though.
This PR was vibe coded with VS Code & Copilot Agent mode. It's fine to close if there's a concurrent effort or something more official. I just wanted to demonstrate it could be done. That said,
cargo testpasses locally 👍